Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positives for variables in font-family-no-missing-generic-family-keyword #5240

Merged
merged 1 commit into from Apr 20, 2021

Conversation

ybiquitous
Copy link
Member

This change aims to ignore any variables (e.g. var(), $var, @var etc.) for the font-family-no-missing-generic-family-keyword rule.

Which issue, if any, is this issue related to?

Fix #4765 (retry of #4806)

Is there anything in the PR that needs further explanation?

This PR ignores always any variables for the font-family property, but perhaps some people might want to ignore explicitly via the ignoreFontFamilies secondary option as below:

{
  "ignoreFontFamilies": [/\$fontFamily/]
}

Could you please give me feedback about this point?

This change aims to ignore any variables (e.g. `var()`, `$var`, `@var` etc.)
for the `font-family-no-missing-generic-family-keyword` rule.

Fix #4765 (retry of #4806)
```css
a { font-family: Helvetica, var(--font-family-common); }
```

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] I think it valuable to add examples which are using variables.

@@ -23,6 +24,12 @@ const messages = ruleMessages(ruleName, {
const isFamilyNameKeyword = (node) =>
!node.quote && keywordSets.fontFamilyKeywords.has(node.value.toLowerCase());

const isLastFontFamilyVariable = (value) => {
const lastValue = postcss.list.comma(value).pop();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] postcss.list.comma() seems more suitable than postcss.list.space() in this case.

See also https://postcss.org/api/#list

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR ignores always any variables for the font-family property, but perhaps some people might want to ignore explicitly via the ignoreFontFamilies secondary option

I think your PR is the right call. Overloading ignoreFontFamily for variables is confusing users, and we should reserve it for its original intention of icon fonts, e.g. font-family: awesomefont; and ignoreFontFamily: ["awesomefont"]

LGTM, thanks.

@jeddy3 jeddy3 changed the title font-family-no-missing-generic-family-keyword ignore any variables Fix false positives for variables in font-family-no-missing-generic-family-keyword Apr 20, 2021
@jeddy3 jeddy3 merged commit 4b7f669 into master Apr 20, 2021
@jeddy3 jeddy3 deleted the issue-4765 branch April 20, 2021 12:30
@jeddy3
Copy link
Member

jeddy3 commented Apr 20, 2021

  • Fixed: font-family-no-missing-generic-family-keyword false positives for variables (#5240).

@ybiquitous
Copy link
Member Author

@jeddy3 Thank you for the review. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix false positives for variables in font-family-no-missing-generic-family-keyword
2 participants